modernize radius filter ... (#511)

* modernize radius filter ...

and fix a memory leak when maxcount option discards wpts.

* radius loop safety.

* allow any Comparable with Waypoint/Route List sort.

* use static_cast instead of reinterpret_cast to

recover original type from void*.

diff --git a/arcdist.cc b/arcdist.cc
index 5eee2a6ca5b9689191112474e750474a5391a275..e75654a17523e2bd8d2e27abcb5b0fc6062baaa1 100644 (file)
--- a/arcdist.cc
+++ b/arcdist.cc
@@ -56,7 +56,7 @@ void ArcDistanceFilter::arcdist_arc_disp_wpt_cb(const Waypoint* arcpt2)
       if (waypointp->extra_data) {
         ed = (extra_data*) waypointp->extra_data;
       } else {
-        ed = (extra_data*) xcalloc(1, sizeof(*ed));
+        ed = new extra_data;
         ed->distance = BADVAL;
       }
       if (ed->distance == BADVAL || projectopt || ed->distance >= pos_dist) {
@@ -206,7 +206,7 @@ void ArcDistanceFilter::process()
                   qPrintable(wp->shortname), ed->distance, wp->latitude, wp->longitude);
         }
       }
-      xfree(ed);
+      delete ed;
     }
   }
   if (global_opts.verbose_status > 0) {

diff --git a/defs.h b/defs.h
index f5dc7ffdc4972ea050fe393c66c0f8504c275306..22d0ec6a362d2b957d1b49e9c63ea3d49332a39a 100644 (file)
--- a/defs.h
+++ b/defs.h
@@ -19,6 +19,7 @@
 #ifndef DEFS_H_INCLUDED_
 #define DEFS_H_INCLUDED_
 
+#include <algorithm>            // for sort, stable_sort
 #include <cmath>                // for M_PI
 #include <cstdarg>              // for va_list
 #include <cstddef>              // for NULL, nullptr_t, size_t
@@ -549,8 +550,6 @@ using waypt_cb = void (*)(const Waypoint*);
 class WaypointList : private QList<Waypoint*>
 {
 public:
-  using Compare = bool (*)(const Waypoint*, const Waypoint*);
-
   void waypt_add(Waypoint* wpt); // a.k.a. append(), push_back()
   void add_rte_waypt(int waypt_ct, Waypoint* wpt, bool synth, const QString& namepart, int number_digits);
   // FIXME: Generally it is inefficient to use an element pointer or reference to define the element to be deleted, use iterator instead,
@@ -565,7 +564,8 @@ public:
   void copy(WaypointList** dst) const;
   void restore(WaypointList* src);
   void swap(WaypointList& other);
-  void sort(Compare cmp);
+  template <typename Compare>
+  void sort(Compare cmp) {std::stable_sort(begin(), end(), cmp);}
   template <typename T>
   void waypt_disp_session(const session_t* se, T cb);
 
@@ -610,7 +610,13 @@ void waypt_append(WaypointList* src);
 void waypt_backup(WaypointList** head_bak);
 void waypt_restore(WaypointList* head_bak);
 void waypt_swap(WaypointList& other);
-void waypt_sort(WaypointList::Compare cmp);
+template <typename Compare>
+void waypt_sort(Compare cmp)
+{
+  extern WaypointList* global_waypoint_list;
+
+  global_waypoint_list->sort(cmp);
+}
 void waypt_add_url(Waypoint* wpt, const QString& link,
                    const QString& url_link_text);
 void waypt_add_url(Waypoint* wpt, const QString& link,
@@ -712,8 +718,6 @@ using route_trl = void (*)(const route_head*);
 class RouteList : private QList<route_head*>
 {
 public:
-  using Compare = bool (*)(const route_head*, const route_head*);
-
   int waypt_count() const;
   void add_head(route_head* rte); // a.k.a. append(), push_back()
   // FIXME: Generally it is inefficient to use an element pointer or reference to define the element to be deleted, use iterator instead,
@@ -729,7 +733,8 @@ public:
   void copy(RouteList** dst) const;
   void restore(RouteList* src);
   void swap(RouteList& other);
-  void sort(Compare cmp);
+  template <typename Compare>
+  void sort(Compare cmp) {std::sort(begin(), end(), cmp);}
   template <typename T1, typename T2, typename T3>
   void disp_all(T1 rh, T2 rt, T3 wc);
   template <typename T2, typename T3>
@@ -795,11 +800,23 @@ void track_append(RouteList* src);
 void route_backup(RouteList** head_bak);
 void route_restore(RouteList* head_bak);
 void route_swap(RouteList& other);
-void route_sort(RouteList::Compare cmp);
+template <typename Compare>
+void route_sort(Compare cmp)
+{
+  extern RouteList* global_route_list;
+
+  global_route_list->sort(cmp);
+}
 void track_backup(RouteList** head_bak);
 void track_restore(RouteList* head_bak);
 void track_swap(RouteList& other);
-void track_sort(RouteList::Compare cmp);
+template <typename Compare>
+void track_sort(Compare cmp)
+{
+  extern RouteList* global_track_list;
+
+  global_track_list->sort(cmp);
+}
 computed_trkdata track_recompute(const route_head* trk);
 
 template <typename T>

diff --git a/radius.cc b/radius.cc
index 49ae52e6414098455b3fd515b17a3da2bbdd125d..e71f2559414e08b665b86c7d69ee52fb87fa80be 100644 (file)
--- a/radius.cc
+++ b/radius.cc
@@ -19,57 +19,29 @@
 
  */
 
-#include <cstdlib>          // for atof, atoi, qsort, strtod
+#include <cstdlib>          // for atoi, strtod
 
 #include <QtCore/QString>   // for QString
-#include <QtCore/QtGlobal>  // for foreach
+#include <QtCore/QtGlobal>  // for qAsConst, QAddConst<>::Type, foreach
 
-#include "defs.h"
+#include "defs.h"           // for Waypoint, waypt_del, route_add_head, route_add_wpt, route_head, waypt_add, waypt_count, xcalloc, xfree, kMilesPerKilometer
 #include "radius.h"
 #include "grtcirc.h"        // for RAD, gcdist, radtomiles
 
+
 #if FILTERS_ENABLED
 
 double RadiusFilter::gc_distance(double lat1, double lon1, double lat2, double lon2)
 {
-  return gcdist(
-           RAD(lat1),
-           RAD(lon1),
-           RAD(lat2),
-           RAD(lon2)
-         );
-}
-
-int RadiusFilter::dist_comp(const void* a, const void* b)
-{
-  const Waypoint* x1 = *(Waypoint**)a;
-  const Waypoint* x2 = *(Waypoint**)b;
-  auto* x1e = (extra_data*) x1->extra_data;
-  auto* x2e = (extra_data*) x2->extra_data;
-
-  if (x1e->distance > x2e->distance) {
-    return 1;
-  }
-  if (x1e->distance < x2e->distance) {
-    return -1;
-  }
-  return 0;
-
+  return radtomiles(gcdist(RAD(lat1), RAD(lon1), RAD(lat2), RAD(lon2)));
 }
 
 void RadiusFilter::process()
 {
-  Waypoint** comp;
-  int i, wc;
-  route_head* rte_head = nullptr;
+  // waypt_del may modify container.
   foreach (Waypoint* waypointp, *global_waypoint_list) {
-    double dist = gc_distance(waypointp->latitude,
-                       waypointp->longitude,
-                       home_pos->latitude,
-                       home_pos->longitude);
-
-    /* convert radians to float point statute miles */
-    dist = radtomiles(dist);
+    double dist = gc_distance(waypointp->latitude, waypointp->longitude,
+                              home_pos->latitude, home_pos->longitude);
 
     if ((dist >= pos_dist) == (exclopt == nullptr)) {
       waypt_del(waypointp);
@@ -77,70 +49,60 @@ void RadiusFilter::process()
       continue;
     }
 
-    auto* ed = (extra_data*) xcalloc(1, sizeof(extra_data));
+    auto* ed = new extra_data;
     ed->distance = dist;
     waypointp->extra_data = ed;
   }
 
-  wc = waypt_count();
-
-  comp = (Waypoint**) xcalloc(wc, sizeof(*comp));
-
-  i = 0;
-
-  /*
-   * Create an array of remaining waypoints, popping them off the
-   * master queue as we go.   This gives us something reasonable
-   * for qsort.
-   */
-
-  foreach (Waypoint* wp, *global_waypoint_list) {
-    comp[i] = wp;
-    waypt_del(wp);
-    i++;
-  }
-
-  if (!nosort) {
-    qsort(comp, wc, sizeof(Waypoint*), dist_comp);
+  if (nosort == nullptr) {
+    auto dist_comp_lambda = [](const Waypoint* a, const Waypoint* b)->bool {
+      const auto* aed = static_cast<const extra_data*>(a->extra_data);
+      const auto* bed = static_cast<const extra_data*>(b->extra_data);
+      return aed->distance < bed->distance;
+    };
+    waypt_sort(dist_comp_lambda);
   }
 
-  if (routename) {
+  route_head* rte_head = nullptr;
+  if (routename != nullptr) {
     rte_head = new route_head;
     rte_head->rte_name = routename;
     route_add_head(rte_head);
   }
 
   /*
-   * The comp array is now sorted by distance.   As we run through it,
-   * we push them back onto the master wp list, letting us pass them
-   * on through in the modified order.
+   * Create an list of remaining waypoints.
+   * Delete them, add them to the global waypoint list, or add them
+   * to a new route.
    */
-  for (i = 0; i < wc; i++) {
-    Waypoint* wp = comp[i];
 
-    xfree(wp->extra_data);
+  WaypointList comp;
+  waypt_swap(comp);
+
+  int i = 0;
+  for (Waypoint* wp : qAsConst(comp)) {
+    delete static_cast<extra_data*>(wp->extra_data);
     wp->extra_data = nullptr;
 
-    if (maxctarg && i >= maxct) {
-      continue;
-    }
-    if (routename) {
-      route_add_wpt(rte_head, wp);
+    if ((maxctarg != nullptr) && (i >= maxct)) {
+      delete wp;
     } else {
-      waypt_add(wp);
+      if (routename != nullptr) {
+        route_add_wpt(rte_head, wp);
+      } else {
+        waypt_add(wp);
+      }
     }
+    ++i;
   }
-
-  xfree(comp);
 }
 
 void RadiusFilter::init()
 {
-  char* fm;
-
   pos_dist = 0;
 
-  if (distopt) {
+  if (distopt != nullptr) {
+    char* fm;
     pos_dist = strtod(distopt, &fm);
 
     if ((*fm == 'k') || (*fm == 'K')) {
@@ -149,7 +111,7 @@ void RadiusFilter::init()
     }
   }
 
-  if (maxctarg) {
+  if (maxctarg != nullptr) {
     maxct = atoi(maxctarg);
   } else {
     maxct = 0;
@@ -157,11 +119,11 @@ void RadiusFilter::init()
 
   home_pos = new Waypoint;
 
-  if (latopt) {
-    home_pos->latitude = atof(latopt);
+  if (latopt != nullptr) {
+    home_pos->latitude = strtod(latopt, nullptr);
   }
-  if (lonopt) {
-    home_pos->longitude = atof(lonopt);
+  if (lonopt != nullptr) {
+    home_pos->longitude = strtod(lonopt, nullptr);
   }
 }
 

diff --git a/radius.h b/radius.h
index ab1b77e41a30681e83abf0c9b8157465719def04..75f5a5ca39bb6dba311d9359691f03eb22a07e6d 100644 (file)
--- a/radius.h
+++ b/radius.h
@@ -89,7 +89,6 @@ private:
   };
 
   double gc_distance(double lat1, double lon1, double lat2, double lon2);
-  static int dist_comp(const void* a, const void* b);
 
 };
 #endif // FILTERS_ENABLED

diff --git a/reference/radius.csv b/reference/radius.csv
index 4095cad3e37c6453526b499b4d628f5bcc378610..f5c656709fe7142402e2071266a74f13b9c08e15 100644 (file)
--- a/reference/radius.csv
+++ b/reference/radius.csv
@@ -1 +1,3 @@
 35.97203, -87.13470, Mountain Bike Heaven by susy1313
+36.05750, -86.89200, GittyUp by JoGPS / Warner Parks
+36.08280, -86.86728, Inlighting by JoGPS / Warner Parks

diff --git a/route.cc b/route.cc
index 3070ea02671bc94d60dfb8fd3f2c46da6ef4c6e4..2cde1145c37f3e3e57afe7c818a84f6934d1856d 100644 (file)
--- a/route.cc
+++ b/route.cc
@@ -211,12 +211,6 @@ route_swap(RouteList& other)
   global_route_list->swap(other);
 }
 
-void
-route_sort(RouteList::Compare cmp)
-{
-  global_route_list->sort(cmp);
-}
-
 void
 track_backup(RouteList** head_bak)
 {
@@ -235,12 +229,6 @@ track_swap(RouteList& other)
   global_track_list->swap(other);
 }
 
-void
-track_sort(RouteList::Compare cmp)
-{
-  global_track_list->sort(cmp);
-}
-
 /*
  * This really makes more sense for tracks than routes.
  * Run over all the trackpoints, computing heading (course), speed, and
@@ -510,8 +498,3 @@ void RouteList::swap(RouteList& other)
   *this = other;
   other = tmp_list;
 }
-
-void RouteList::sort(Compare cmp)
-{
-  std::sort(begin(), end(), cmp);
-}

diff --git a/testo.d/radius.test b/testo.d/radius.test
index 70c3a400c7dbd1359467cf7f2f8204ae40e44d93..41c19c788ca0b30f870fb26fe4b2cb87589d3e09 100644 (file)
--- a/testo.d/radius.test
+++ b/testo.d/radius.test
@@ -4,7 +4,6 @@
 #
 rm -f ${TMPDIR}/radius.csv
 gpsbabel -i geo -f ${REFERENCE}/geocaching.loc \
-               -x radius,lat=35.9720,lon=-87.1347,distance=14.7 \
+               -x radius,lat=35.9720,lon=-87.1347,distance=20.0,maxcount=3 \
                -o csv -F ${TMPDIR}/radius.csv
 compare ${TMPDIR}/radius.csv ${REFERENCE}
-

diff --git a/waypt.cc b/waypt.cc
index e7e942b8634ad530b3e15c58ab75ca7f553f0acb..15cfe3493c19cb5cdfbfb9db9a747f5466e88f1f 100644 (file)
--- a/waypt.cc
+++ b/waypt.cc
@@ -197,12 +197,6 @@ waypt_swap(WaypointList& other)
   global_waypoint_list->swap(other);
 }
 
-void
-waypt_sort(WaypointList::Compare cmp)
-{
-  global_waypoint_list->sort(cmp);
-}
-
 void
 waypt_add_url(Waypoint* wpt, const QString& link, const QString& url_link_text)
 {
@@ -745,8 +739,3 @@ void WaypointList::swap(WaypointList& other)
   *this = other;
   other = tmp_list;
 }
-
-void WaypointList::sort(Compare cmp)
-{
-  std::stable_sort(begin(), end(), cmp);
-}